Skip to content

Fixing asan error on mpmc queue#244

Open
MaciejKaszynski wants to merge 2 commits into
eclipse-score:mainfrom
etas-contrib:fix-asan-queue-error
Open

Fixing asan error on mpmc queue#244
MaciejKaszynski wants to merge 2 commits into
eclipse-score:mainfrom
etas-contrib:fix-asan-queue-error

Conversation

@MaciejKaszynski

@MaciejKaszynski MaciejKaszynski commented Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes the ASAN error with the MPMCConcurrentQueue

Using manual padding instead of alignas so that ASAN's fake stack can correctly.
Setting the alignment on the class itself didn't fix the asan error as it would just set the alignment for the class itself.

@github-actions

github-actions Bot commented Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run --lockfile_mode=error //:license-check

Status: ⚠️ Needs Review

Click to expand output 
[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.4.2) and connecting to it...
INFO: Invocation ID: 27b2b7e9-a010-4a1d-9b2a-b5c0f91f65f6
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //:license-check (30 packages loaded, 10 targets configured)

Analyzing: target //:license-check (81 packages loaded, 10 targets configured)

Analyzing: target //:license-check (86 packages loaded, 10 targets configured)

Analyzing: target //:license-check (138 packages loaded, 2720 targets configured)

Analyzing: target //:license-check (147 packages loaded, 6526 targets configured)

Analyzing: target //:license-check (153 packages loaded, 8081 targets configured)

Analyzing: target //:license-check (154 packages loaded, 8081 targets configured)

Analyzing: target //:license-check (158 packages loaded, 10210 targets configured)

INFO: Analyzed target //:license-check (163 packages loaded, 10268 targets configured).
[12 / 16] JavaToolchainCompileClasses external/rules_java+/toolchains/platformclasspath_classes; 0s disk-cache, processwrapper-sandbox ... (2 actions, 1 running)
[14 / 16] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar; 0s disk-cache, processwrapper-sandbox
[15 / 16] Building license.check.license_check.jar (); 0s disk-cache, multiplex-worker
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 23.707s, Critical Path: 2.51s
INFO: 16 processes: 12 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 16 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown

The created documentation from the pull request is available at: docu-html

@VukPavicRTRK VukPavicRTRK self-requested a review June 11, 2026 12:11
VukPavicRTRK
VukPavicRTRK suggested changes Jun 11, 2026
View reviewed changes

@VukPavicRTRK VukPavicRTRK left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix for the ASan false positive. One thing to call out: CacheLinePaddedAtomic<T> preserves cache-line size but not cache-line alignment, so it weakens the original false-sharing guarantee provided by alignas(CacheLineSize). At minimum, the comments on m_head, m_tail, and m_stopped should be updated since they're no longer strictly "aligned on their own cache line."

@MaciejKaszynski

MaciejKaszynski commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Nice fix for the ASan false positive. One thing to call out: CacheLinePaddedAtomic<T> preserves cache-line size but not cache-line alignment, so it weakens the original false-sharing guarantee provided by alignas(CacheLineSize). At minimum, the comments on m_head, m_tail, and m_stopped should be updated since they're no longer strictly "aligned on their own cache line."

So I have done a benchmark and seen that false sharing doesn't actually cause any performance drawbacks. I have committed the benchmark and made the members not aligned at all .

@S-MOHAMD

S-MOHAMD commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Please lnk corresponding issue/bug.
see: https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@VukPavicRTRK VukPavicRTRK VukPavicRTRK requested changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@MaciejKaszynski MaciejKaszynski

Labels

None yet

Projects

LCM - Lifecycle & Health FT
Status: Backlog

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

mpmc_concurrent_queue_test excluded from ASan/UBSan due to fake-stack alignment false positive

4 participants

@MaciejKaszynski @S-MOHAMD @VukPavicRTRK @anmittag